Inventory firmware #261

joelrebel · 2022-04-07T15:07:00Z

Checklist

Tests added
Similar commits squashed
What does this PR implement?
- Defines and implements a InventoryGetter interface for Redfish, Asrockrack
- Defines and implements a FirmwareInstaller, FirmwareInstallVerifier interfaces
- Defines and implements a PostCodeGetter interface for Asrockrack
- Implements the PowerStateGetter, PowerState setter interfaces for Asrockrack
The HW vendor this change applies to
- Asrockrack, Redfish supported hardware
The HW model number, product name this change applies to
- Asrockrack - E3C246D4I-NL
What version of tooling - vendor specific or opensource does this change depend on
- Gofish v0.12.1
Description for changelog/release notes
- Defines and implements a InventoryGetter interface for Redfish, Asrockrack
- Defines and implements a FirmwareInstaller, FirmwareInstallVerifier interfaces
- Defines and implements a PostCodeGetter interface for Asrockrack
- Implements the PowerStateGetter, PowerState setter interfaces for Asrockrack

jacobweinstock

Sorry, this is quite a large PR. Might take me some time to get through.

bmc/inventory.go

joelrebel · 2022-04-19T06:06:27Z

@jacobweinstock thanks for taking a look at this! let me know when you're done and I can cut a v0.5.3 release.
I have a few more changes that I'll drop into a sub branch, sorry about the size of this PR, I can work on having smaller stacked PRs instead going ahead for related changes.

micahhausler

I haven't programmed against many of these BMCs, but have a few suggestions on the PR

bmc/firmware.go

bmc/inventory.go

bmc/postcode.go

devices/constants.go

providers/redfish/firmware.go

providers/redfish/firmware_test.go

providers/redfish/inventory.go

joelrebel · 2022-04-22T06:05:54Z

@micahhausler thanks for the review!

micahhausler

Nice cleanup. I have just a few minor suggestions

bmc/firmware.go

micahhausler · 2022-04-22T14:04:12Z

providers/asrockrack/firmware.go

+// FirmwareInstall uploads and initiates firmware update for the component
+func (a *ASRockRack) FirmwareInstall(ctx context.Context, component, applyAt string, forceInstall bool, reader io.Reader) (jobID string, err error) {
+	var size int64
+	if file, ok := reader.(*os.File); ok {


If length is required, maybe consider a different interface like fs.File or you could declare an interface like

type LengthReader interface { io.Reader Len() int }

which is satisfied by bytes.Buffer and strings.Reader and provide a helper for os.File/fs.File like

type lr struct { fs.File } func (lr *r) Len() int { info, err := r.Stat() if err != nil { panic(err) // or ignore/log } return int(info.Size()) } // LengthReaderFromFile returns a LengthReader from an fs.File func LengthReaderFromFile(f fs.File) LengthReader { return &lr{f} }

micahhausler · 2022-04-22T14:06:43Z

providers/asrockrack/firmware.go

+	a.log.V(2).Info("info", "action", "set device to flash mode, takes a minute...", "step", "1/4")
+	err = a.setFlashMode(ctx)
+	if err != nil {
+		return fmt.Errorf("failed in step 1/4 - set device to flash mode: " + err.Error())


Why not use errors.Wrap()? That would allow callers to use errors.Is(). Same for the other steps in this file

Thanks for the suggestion here, as of now the file size is required for the multipart upload. I've created a separate issue to follow up on the LengthReader interface #261 (comment)

Fixed up errors Wrap in aa25390

providers/redfish/inventory.go

micahhausler

LGTM, needs a rebase

- include example

… dependency

…wareInstallVerifier

…rifier interface methods

…stallVerifier interface methods - use context for all requests - implement various helper methods for inventory, firmware updates - split up firmware install, status methods - add tests

…terfaces

… FirmwareInstallUnknown this is so that the caller is aware that the BMC is not aware of the component version and can decide if it wants to power cycle or take other actions. instead of assuming the device needs a powercycle

…payload this is an attempt to have the BMC preserve the User, Network BMC configuration after a flash

…eset error returned when the install status lookup fails

This enables the ASRR and Redfish providers to reset the BMC

micahhausler

looking great! Just a few minor changes

examples/v1/firmware/firmware.go

providers/asrockrack/firmware.go

codecov · 2022-04-26T14:54:29Z

Codecov Report

Merging #261 (eb3610c) into master (2af4651) will increase coverage by 0.21%.
The diff coverage is 37.41%.

@@            Coverage Diff             @@
##           master     #261      +/-   ##
==========================================
+ Coverage   23.73%   23.94%   +0.21%     
==========================================
  Files          59       70      +11     
  Lines       10276    11425    +1149     
==========================================
+ Hits         2439     2736     +297     
- Misses       7300     8094     +794     
- Partials      537      595      +58

Impacted Files	Coverage Δ
client.go	`0.00% <0.00%> (ø)`
providers/asrockrack/power.go	`0.00% <0.00%> (ø)`
providers/redfish/inventory_collect.go	`0.00% <0.00%> (ø)`
providers/redfish/redfish.go	`7.69% <8.33%> (ø)`
providers/redfish/inventory.go	`8.55% <8.55%> (ø)`
providers/asrockrack/firmware.go	`10.18% <10.18%> (ø)`
providers/redfish/firmware.go	`43.44% <43.44%> (ø)`
providers/asrockrack/asrockrack.go	`44.89% <57.14%> (+12.78%)`	⬆️
providers/asrockrack/helpers.go	`47.58% <63.88%> (+1.95%)`	⬆️
providers/redfish/tasks.go	`69.86% <69.86%> (ø)`
... and 9 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0cb1bdd...eb3610c. Read the comment docs.

- moves the optional component parameter at the end - providers/asrockrack/firmware: wrap FirmwareInstall errors

joelrebel · 2022-04-26T15:44:43Z

rebased on master and squashed a few similar commits

micahhausler

LGTM, no blocking changes

examples/v1/inventory/inventory.go

providers/asrockrack/asrockrack.go

also fixes the registrar.Verifier implementation

joelrebel requested review from jacobweinstock and mikemrm April 7, 2022 15:07

jacobweinstock reviewed Apr 12, 2022

View reviewed changes

bmc/inventory.go Outdated Show resolved Hide resolved

jacobweinstock mentioned this pull request Apr 12, 2022

Deleted tags - Cut a v0.5.3 release #262

Closed

joelrebel requested a review from diogomatsubara April 20, 2022 13:04

micahhausler suggested changes Apr 20, 2022

View reviewed changes

joelrebel force-pushed the inventory-firmware branch from 04afa2d to aac60b0 Compare April 22, 2022 05:27

joelrebel mentioned this pull request Apr 22, 2022

Power states as type aliases #266

Open

joelrebel force-pushed the inventory-firmware branch from aac60b0 to 623847d Compare April 22, 2022 05:44

joelrebel mentioned this pull request Apr 22, 2022

Adds BmcReset methods and few fixes for ASRR firmware installed version checks #267

Merged

2 tasks

micahhausler reviewed Apr 22, 2022

View reviewed changes

joelrebel mentioned this pull request Apr 25, 2022

FirmwareInstall method could wrap io.Reader for file length #268

Open

micahhausler approved these changes Apr 25, 2022

View reviewed changes

joelrebel added 15 commits April 26, 2022 12:13

define various consts, errors, providers, device types

430ebdf

bmc/inventory: define inventory getter interface and method

683e135

providers/asrockrack: implement InventoryGetter interface

7e7ce13

- include example

providers/redfish: implement InventoryGetter interface, update gofish…

3de88c2

… dependency

bmc/firmware: replace previous interface with FirmwareInstaller, Firm…

440f699

…wareInstallVerifier

providers/redfish: implement FirmwareInstallStatus, FirmwareInstallVe…

7ab7659

…rifier interface methods

providers/asrockrack: rework, implement FirmwareInstaller, FirmwareIn…

b061360

…stallVerifier interface methods - use context for all requests - implement various helper methods for inventory, firmware updates - split up firmware install, status methods - add tests

providers/asrockrack: implement PowerStateGetter, PowerStateSetter in…

3bddf1d

…terfaces

bmc/postcode: define interfaces to collect BIOS/UEFI POST codes

df48b12

bmc/client: define methods for Inventory, Firmware, PostCode actions

d2657e4

rename getter methods

a73cdcc

Implement changes based on feedback from GH PR#261

34c2169

providers/asrockrack: include various other fields in the BMC update …

84306c7

…payload this is an attempt to have the BMC preserve the User, Network BMC configuration after a flash

consts, errors: firmware install status returned when a BMC needs a r…

f09df96

…eset error returned when the install status lookup fails

joelrebel added 2 commits April 26, 2022 12:20

providers/asrr, redfish: implement BmcReset interface

201a037

This enables the ASRR and Redfish providers to reset the BMC

redfish: fix implementation, rename GetInventory() -> Inventory()

95d0197

micahhausler suggested changes Apr 26, 2022

View reviewed changes

joelrebel force-pushed the inventory-firmware branch from 3c070f6 to ca6978d Compare April 26, 2022 14:52

joelrebel added 2 commits April 26, 2022 17:27

bmc/firmware: update FirmwareInstallStatus() method signature

91fc904

- moves the optional component parameter at the end - providers/asrockrack/firmware: wrap FirmwareInstall errors

redfish/inventory: split out helper methods

e2543e3

joelrebel force-pushed the inventory-firmware branch from ca6978d to 04ed8e8 Compare April 26, 2022 15:28

joelrebel added 4 commits April 26, 2022 17:32

providers/asrockrack: use structured logging, purge debug statements

a68ebce

examples: merge changes from master, fixup

8e5a784

providers: merge changes from master, a few suggested fixes

0f7a4c7

go.mod: deps updated after rebase

92f6fc0

joelrebel force-pushed the inventory-firmware branch from 04ed8e8 to 92f6fc0 Compare April 26, 2022 15:37

joelrebel requested a review from micahhausler April 26, 2022 15:44

micahhausler approved these changes Apr 27, 2022

View reviewed changes

examples/v1/inventory/inventory.go Outdated Show resolved Hide resolved

providers/asrockrack/asrockrack.go Outdated Show resolved Hide resolved

micahhausler added the ready-to-merge label Apr 27, 2022

micahhausler removed request for diogomatsubara and mikemrm April 27, 2022 15:57

joelrebel added 2 commits April 27, 2022 17:59

providers/asrockrack: pass context to Compatible()

b14936a

also fixes the registrar.Verifier implementation

examples/v1/inventory: fix JSON indent

eb3610c

micahhausler approved these changes Apr 27, 2022

View reviewed changes

joelrebel merged commit f660df4 into master Apr 27, 2022

joelrebel deleted the inventory-firmware branch July 26, 2022 09:58

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Inventory firmware #261

Inventory firmware #261

joelrebel commented Apr 7, 2022

jacobweinstock left a comment

joelrebel commented Apr 19, 2022

micahhausler left a comment

joelrebel commented Apr 22, 2022

micahhausler left a comment

micahhausler Apr 22, 2022 •

edited

Loading

micahhausler Apr 22, 2022

joelrebel Apr 25, 2022

micahhausler left a comment

micahhausler left a comment

codecov bot commented Apr 26, 2022 •

edited

Loading

joelrebel commented Apr 26, 2022

micahhausler left a comment

Inventory firmware #261

Inventory firmware #261

Conversation

joelrebel commented Apr 7, 2022

jacobweinstock left a comment

Choose a reason for hiding this comment

joelrebel commented Apr 19, 2022

micahhausler left a comment

Choose a reason for hiding this comment

joelrebel commented Apr 22, 2022

micahhausler left a comment

Choose a reason for hiding this comment

micahhausler Apr 22, 2022 • edited Loading

Choose a reason for hiding this comment

micahhausler Apr 22, 2022

Choose a reason for hiding this comment

joelrebel Apr 25, 2022

Choose a reason for hiding this comment

micahhausler left a comment

Choose a reason for hiding this comment

micahhausler left a comment

Choose a reason for hiding this comment

codecov bot commented Apr 26, 2022 • edited Loading

Codecov Report

joelrebel commented Apr 26, 2022

micahhausler left a comment

Choose a reason for hiding this comment

micahhausler Apr 22, 2022 •

edited

Loading

codecov bot commented Apr 26, 2022 •

edited

Loading